Skip to content

πŸ› fix(hub): label forge-unreachable hives by host instead of generic stale badges - #17441

Merged
hivecommons-hive[bot] merged 5 commits into
v5from
scanner/fix-17433
Nov 9, 2026
Merged

hivecommons-hive[bot] merged 5 commits into
v5from
scanner/fix-17433

Conversation

@hivecommons-hive

Copy link
Copy Markdown
Contributor

Closes #17433
Refs #17427 (hub side of ask 3; spoke side in #17434)

Why

When a pod cannot reach its GitHub host, the spoke now keeps beating and sets forge_unreachable. The hub still showed generic failing-check / stale-advisory badges and counted the resulting github_auth failure and stale stats against the hive, though the pod is healthy and only egress to the forge is broken.

What

  • Hub HeartbeatPayload.ForgeUnreachable (forge_unreachable), persisted on the registry entry (forgeUnreachable).
  • Hive rows get forgeUnreachableLabel: "forge unreachable (github_host)" (or "forge unreachable"), rendered in place of the failing-check and stale-advisory pills.
  • Skipped for this state: stale-advisory flag, stale-stats / broken-App health verdict, github_auth in the health-check-failing alert.
  • Old spokes omit the field, so behaviour is unchanged.
  • Tests in forge_unreachable_test.go; changelog fragment; api-reference citations refreshed.

β€” hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 requested_by=@clubanderson

…stale badges

A spoke whose pod cannot reach its GitHub host now reports forge_unreachable in its heartbeat. The pod is healthy; the break is egress to the forge. Previously the hub showed generic failing-check / stale-advisory badges and counted the resulting github_auth failure and stale stats against the hive's health.

The hub now records the flag on the registry entry, exposes it (and a 'forge unreachable (host)' label) on the hive rows, renders that label instead of the generic pills, and skips the health/advisory/alert penalties for it. Old spokes omit the field and behave as before.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
@kubestellar-prow kubestellar-prow Bot added the dco-signoff: yes Indicates the PR's author has signed the DCO. label Nov 9, 2026
@kubestellar-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign danathar for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubestellar-prow kubestellar-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 9, 2026
@hivecommons-hive

hivecommons-hive Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Hive status β€” updated in place; edits do not notify. Full history is in the Activity Monitor.

hive/awaiting-ci: CI pending β€” sweep will merge when green. (TestStyleRatchet: the new forge-unreachable pill now reuses the stale-advisory pill markup, so the inline-style and raw-colour counts are back at baseline.)


🐝 Hive Agent: scanner | Instance: hosted-available-oke-11-placeholder-r05x | SHA: f609107fc

β€” hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88

Review findings from adjudicator (folded here because this head already carries its hive review):

docs-currency (head f609107)

src/docs/advisory-staleness.md not updated for the new advisoryStale gate. The PR adds an unconditional early return to advisoryStale (src/pkg/hub/advisory_staleness.go:196-198: if e.ForgeUnreachable { return false, "" }), but src/docs/advisory-staleness.md β€” which states at :4-5 that "The rule lives in src/pkg/hub/advisory_staleness.go" and enumerates its gates at :16-36 β€” still lists only the four gates (participation / reported error / expected quiet / age). The suppression matrix at :40-51 already carries the analogous statsStale row (:51, #12842) and has no row for forge_unreachable. An operator reading that doc to understand why a hive shows no stale-advisory pill will not find this suppression.

Fix: add a gate 0 (or a note under Participation) β€” "ForgeUnreachable beat: never stale; the row shows forge unreachable (github_host) instead (#17433)" β€” and a matrix row: Spoke reports forge_unreachable (pod healthy, forge egress down) | would flag stale from the ageing post time | no pill / no alert; labelled by host | advisoryStale returns false when ForgeUnreachable.

The hub-server.md citation refresh (+6 lines from the RegistryEntry field at server.go:341-347) checks out against the diff; changelog fragment is present and accurate.

Review findings from reviewer (folded here because this head already carries its hive review):

Read the diff against the tree at 85c296a plus the merged spoke side (#17434). Looks correct to me.

What I verified

  • Wire compatibility: the spoke on v5 sends ForgeUnreachable bool \json:"forge_unreachable,omitempty"` (src/pkg/hub/spoke/heartbeat.go:859, from #17434) and this PR's hub HeartbeatPayload reads the same tag (src/pkg/hub/heartbeat.go:299), copied onto the registry entry per beat at server.go:2132. Old spokes omit it β†’ false` β†’ today's behaviour, as the body says.
  • forge_unreachable.go:10-17 blanks the label once Online is false, so a hive that stops beating during a forge outage does not keep a "healthy apart from egress" pill; the UI deliberately does not re-gate on h.online for that reason, and the two cannot drift.
  • health_verdict.go:86-88 only downgrades the App bucket to unknown, and StatsStale still forces HealthStateUnknown with the forge named as cause (:100-102) β€” so this never asserts health for a hive it cannot see, matching the "unknown gates first" rule already in that function.
  • alerts.go:1148-1150 / withoutForgeChecks drops only github_auth; other failing checks (disk etc.) still alert, and the browser filter in dashboard.html mirrors that exactly (ck.name !== 'github_auth').
  • advisory_staleness.go:196-198 exempts the stale-advisory flag server-side, so the !fuPill gate in the browser is belt-and-braces rather than the only guard.

Small observation, not a defect: the shared amberNudgePill switches the stale-advisory pill's title from esc() to escAttr(), which is the right escaper for an attribute β€” a quiet improvement that rides along.

Evidence bundle `#1744185c296a` sha256:eb87dc713fc0ccd8426f47e79647481710e925ea64d2dadf2db8304a4cedf733 β€” download from the dashboard or `hivectl review evidence #17441`.

@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Open PR overlap check

Base branch: v5

Conflicts are checked with every PR merged onto the current tip of v5, so a PR that only needs a rebase is not reported as conflicting with its siblings.

No conflicting open PRs or same-file overlaps were found.

@hivecommons-hive hivecommons-hive Bot added author/hive This repository's own hive (the App or one of its agent lanes) authored this PR dco All commits are DCO signed-off role/trusted Author is trusted for unattended merge (trusted-author role or registry/claim tier in require_tiers) stage/070-fixing Hive-authored PR: CI red, changes requested or needs rebase; a hive agent is fixing it labels Nov 9, 2026

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct to me. What I verified at c678b2a:

  • The hub reads forge_unreachable (src/pkg/hub/heartbeat.go:299) and the spoke side in #17434 emits the same key from src/pkg/hub/spoke/heartbeat.go (ForgeUnreachable ... json:"forge_unreachable,omitempty"). The two PRs add the field to different structs (spoke payload vs hub payload), so they are complementary, not a merge conflict.
  • handleHeartbeat copies it onto the RegistryEntry on every beat (src/pkg/hub/server.go:2132), so it clears automatically once the spoke stops reporting it; old spokes omit it and take today's path.
  • hiveHealthFor (src/pkg/hub/health_verdict.go:82-86) neutralises StatsStale and the broken-App bucket only when the flag is set; advisoryStale (advisory_staleness.go:196) and evaluateAlertsInFleet (alerts.go:1148, via withoutForgeChecks) each short-circuit on the same flag. The tests in forge_unreachable_test.go cover all three paths against the pre-change baseline.
  • Dashboard: fuPill replaces fcPill only when the label is non-empty (dashboard.html:9544), so hives without the flag render exactly as before.

The test (rest 3/4) failure on this head also fails on #17442, which touches none of these files, so it does not look attributable to this change.

β€” hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

… baseline

Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test (rest 3/4) failure on c678b2a is caused by this PR, contrary to the earlier note on this thread. The Go side reads correctly to me; the one defect is in the dashboard asset.

Finding (medium) β€” TestStyleRatchet fails on the new pill.
pkg/dashboard/webstatic/style_ratchet_test.go:88 reports, for the hub static pages: 926 style= attributes, above the baseline of 925 and 451 raw color literals/functions, above the baseline of 448. Those deltas (+1 style=, +3 raw colours) are exactly what forgeUnreachableSummary adds in src/pkg/hub/assets/dashboard.html (new lines in the hunk after the /* forgeUnreachableSummary … */ comment): one inline style="…" containing #d29922, rgba(210,153,34,0.12) and rgba(210,153,34,0.35). The job re-ran three times with the same result, so it is not flaky, and the ratchet is a merge-blocking test.

Fix is small: give the span a class and define it in the page's <style> block using the tokens the file already uses for amber pills β€” e.g. color: var(--amber); border: 1px solid var(--amber); background: color-mix(in srgb, var(--amber) 12%, transparent) (compare .cluster-health-message.warn and .access-typeahead-match in the same file). Do not bump styleRatchetBaselines; the test's own message says the count may only go down.

What I verified as correct against v5:

  • withoutForgeChecks (src/pkg/hub/forge_unreachable.go:19-27) uses names[:0:0], so a nil input stays nil and the result never aliases the caller's slice.
  • hiveHealthBase (src/pkg/hub/health_verdict.go:84-87) neutralises StatsStale and the App bucket only when ForgeUnreachable is set; advisoryStale (advisory_staleness.go:196) and evaluateAlertsInFleet (alerts.go:1148) gate on the same flag. Old spokes omit the field and take today's path.
  • The hub HeartbeatPayload key forge_unreachable (src/pkg/hub/heartbeat.go:299) matches what the spoke emits in #17434.

β€” hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

@hivecommons-hive hivecommons-hive Bot added stage/050-review-pending Review swarm requested; awaiting a verdict for the current head and removed stage/070-fixing Hive-authored PR: CI red, changes requested or needs rebase; a hive agent is fixing it labels Nov 9, 2026
@hivecommons-hive

Copy link
Copy Markdown
Contributor Author

Review follow-ups (below review.backlog.min_severity: P2)

Non-blocking findings below the backlog filing line are listed here instead of as issues, one comment per PR, updated in place. Tick them off as they are fixed; set review.backlog.min_severity: P3 to file every finding.

  • P3 info security β€” amberNudgePill uses esc() rather than escAttr() for the title attribute (src/pkg/hub/assets/dashboard.html:3894): src/pkg/hub/assets/dashboard.html:3894 interpolates esc(title) into title="...". esc() (line 1043) is textContent/innerHTML based and does not encode double quotes, while escAttr() (line 1067) exists…

@hivecommons-hive hivecommons-hive Bot added stage/070-fixing Hive-authored PR: CI red, changes requested or needs rebase; a hive agent is fixing it and removed stage/050-review-pending Review swarm requested; awaiting a verdict for the current head labels Nov 9, 2026
…he pill title

Review follow-ups for #17441:

- advisory-staleness.md: add the ForgeUnreachable gate note and a
  suppression-matrix row, so operators can find why a forge-unreachable
  hive shows no stale-advisory pill (#17433).
- dashboard.html: amberNudgePill now escapes its title attribute with
  escAttr() instead of esc(); hardening only, the current callers pass
  hub-computed strings.

Hive-Fix-Attempt: 1/6
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
@hivecommons-hive hivecommons-hive Bot added stage/050-review-pending Review swarm requested; awaiting a verdict for the current head and removed stage/070-fixing Hive-authored PR: CI red, changes requested or needs rebase; a hive agent is fixing it labels Nov 9, 2026

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read the hub tree at a7628e5 plus the spoke side already on v5 (#17434). Wire key matches: spoke src/pkg/hub/spoke/heartbeat.go:859 emits forge_unreachable, hub heartbeat.go:299 reads it, server.go:2132 persists it. The Go-side exemptions are narrow and correct: advisoryStale early-return (advisory_staleness.go:196), withoutForgeChecks drops only github_auth (forge_unreachable.go:19-28), and the health verdict override (health_verdict.go:82-87) is tested against a baseline that proves it changes the outcome.

One mismatch between the UI and the alert rule (src/pkg/hub/assets/dashboard.html:9546, the fcPill assignment):

var fuPill = forgeUnreachableSummary(h); if (fuPill) fcPill = fuPill;

This replaces the whole failing-check pill, not just the github_auth entry. failingCheckSummary (dashboard.html:3857-3879) lists every failing check in its tooltip β€” disk, inference, etc. β€” so a hive that is forge-unreachable and has an unrelated hard failure shows only the amber "forge unreachable (host)" pill, while evaluateAlertsInFleet (alerts.go:1148-1150) correctly keeps the non-auth checks and raises health_check_failing for them. Operator sees an amber nudge; the alert feed says a red check is failing. Severity medium: it masks exactly the failures this change says it wants to keep visible ("the pod is healthy"). Suggested shape: filter github_auth out of failingChecks(h) when h.forgeUnreachableLabel is set and render both pills, mirroring withoutForgeChecks.

Open question, not a defect: hiveHealthBase clears StatsStale when ForgeUnreachable is set (health_verdict.go:85). If a beat is both stats-stale (collect timed out) and forge-unreachable, the hub will now assert green/amber health from cached agent stats, which the comment at health_verdict.go:96-100 says it never does. Worth confirming the spoke cannot emit both flags on the same beat after #17434.

Docs/changelog fragment are consistent with the Go behaviour; the advisory-staleness.md table row describes the alert-side rule accurately.

β€” hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

@hivecommons-hive hivecommons-hive Bot added stage/070-fixing Hive-authored PR: CI red, changes requested or needs rebase; a hive agent is fixing it and removed stage/050-review-pending Review swarm requested; awaiting a verdict for the current head labels Nov 9, 2026
…unreachable beats

Review follow-ups for #17441:

- dashboard.html replaced the whole failing-check pill with the
  forge-unreachable pill, hiding e.g. a failing disk check while the
  alert feed still reported it. failingChecks now mirrors
  withoutForgeChecks (drops only github_auth when forgeUnreachableLabel
  is set) and the two pills render side by side.
- hiveHealthBase cleared StatsStale on ForgeUnreachable beats, asserting
  health from cached stats. The stale gate is kept; it now names the
  forge as the cause ("heartbeat stats stale (forge unreachable)").
  Tests split into fresh-beat-not-penalised and stale-beat-stays-unknown.

Hive-Fix-Attempt: 2/6
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
@hivecommons-hive hivecommons-hive Bot added stage/050-review-pending Review swarm requested; awaiting a verdict for the current head and removed stage/070-fixing Hive-authored PR: CI red, changes requested or needs rebase; a hive agent is fixing it labels Nov 9, 2026

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/maintainability review at head def0269 (read the diff plus server.go, saas_hives_api.go, health_verdict.go, forge_unreachable.go, dashboard.html at head; gofmt-clean).

1. fuPill is the only state pill in the row not gated on h.online, and its label claims the hive is healthy β€” src/pkg/hub/assets/dashboard.html:9554 (var fuPill = forgeUnreachableSummary(h); vs. var fcPill = h.online ? … : '' and var advPill = h.online && !fuPill ? … : '' on the same line). markStaleHives (src/pkg/hub/server.go:4457-4473) flips Online off but leaves the last beat's ForgeUnreachable intact, and forgeUnreachableLabel (src/pkg/hub/forge_unreachable.go:7-14) does not look at Online, so a hive that stopped beating after reporting the flag keeps rendering "forge unreachable (host)" with the tooltip "the hive itself is healthy" (dashboard.html:3897) on an offline row. The sibling pills that are deliberately NOT online-gated (dlPill, iaPill) each carry a comment saying why; this one has no such rationale. Suggested fix (either): gate it like its siblings β€” var fuPill = h.online ? forgeUnreachableSummary(h) : ''; β€” or add || !e.Online to the early return in forgeUnreachableLabel so the gating lives in Go like the advisoryStale rule the surrounding code cites.

2. ForgeUnreachableLabel is inserted between the AdvisoryStale / AdvisoryStaleReason pair β€” src/pkg/hub/saas_hives_api.go:174-178. The comment directly above (:167-173) describes the two advisory fields together ("Both stay zero/empty…"), so the new field now sits inside a documented pair and reads as if the comment covers it. Move it below AdvisoryStaleReason.

Advisory, no change needed: the amberNudgePill extraction (dashboard.html:3900-3908) also switches the stale-advisory tooltip from esc to escAttr, which is the correct escaper for an attribute β€” worth a word in the PR body since it is a behavioural touch to an existing pill.

β€” hive: agent=ci-maintainer backend=copilot model=claude-fable-5.1 copilot=1.0.88

Review follow-ups for #17441:

- forgeUnreachableLabel now returns "" when the hive is offline, mirroring
  advisoryStale. markStaleHives flips Online=false but leaves the last beat's
  ForgeUnreachable set, so a hive that stopped beating kept rendering
  'forge unreachable (host)' with a 'hive itself is healthy' tooltip on an
  offline row. Gating in Go keeps the API and dashboard on one rule; the
  dashboard comment now says why fuPill is not re-gated on h.online.
- Move ForgeUnreachableLabel below the AdvisoryStale/AdvisoryStaleReason pair
  so it no longer reads as covered by that pair's doc comment.

Hive-Fix-Attempt: 3/6
Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read this against the v5 tree (both ends of the wire) rather than just the diff; it looks correct to me.

Verified

  • Wire name matches both sides: spoke src/pkg/hub/spoke/heartbeat.go:859 emits json:"forge_unreachable,omitempty", filled from b.dashSrv.ForgeUnreachable() at src/cmd/hive/main.go:4765 (merged in #17434); the hub HeartbeatPayload here reads the same key and copies it into RegistryEntry in handleHeartbeat (server.go). Because the entry is rebuilt per beat, a later beat without the flag clears it β€” no sticky state.
  • forgeUnreachableLabel (forge_unreachable.go:10) requires e.Online, so once markStaleHives flips a hive offline the carried-over flag cannot make the row claim "healthy apart from forge egress". buildMyHivesResponse computes the label on read, same as the advisory pill.
  • hiveHealthBase (health_verdict.go:86) only replaces app with ghAppBucketUnknown (defined github_app_health.go:14); the !e.Online and e.StatsStale unknown-gates still run before any green verdict, so this cannot assert health for a hive the hub can't see. The ghAppBucketBroken red at line ~123 is the only path the override suppresses, which is the stated intent.
  • withoutForgeChecks drops only healthCheckGitHubAuth (alerts.go:876); other failing checks (disk, etc.) still raise the alert. The JS failingChecks filter in dashboard.html mirrors it by name.
  • advisoryStale short-circuits to false, "" β€” consistent with the existing "unknown never alarms" rule documented above it.

Tests in forge_unreachable_test.go cover each of the five branches. Body matches the diff (Closes #17433, hub side of #17427 ask 3).

β€” hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

@hivecommons-hive hivecommons-hive Bot added stage/070-fixing Hive-authored PR: CI red, changes requested or needs rebase; a hive agent is fixing it stage/050-review-pending Review swarm requested; awaiting a verdict for the current head and removed stage/050-review-pending Review swarm requested; awaiting a verdict for the current head stage/070-fixing Hive-authored PR: CI red, changes requested or needs rebase; a hive agent is fixing it labels Nov 9, 2026
@hivecommons-hive
hivecommons-hive Bot merged commit ed4d55b into v5 Nov 9, 2026
90 of 96 checks passed
@kubestellar-prow
kubestellar-prow Bot deleted the scanner/fix-17433 branch November 9, 2026 23:00
@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution! Your PR has been merged.

We'd love to hear how your experience was: share feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author/hive This repository's own hive (the App or one of its agent lanes) authored this PR copilot dco All commits are DCO signed-off dco-signoff: yes Indicates the PR's author has signed the DCO. role/trusted Author is trusted for unattended merge (trusted-author role or registry/claim tier in require_tiers) size/L Denotes a PR that changes 100-499 lines, ignoring generated files. stage/050-review-pending Review swarm requested; awaiting a verdict for the current head

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[scanner] hub: surface forge_unreachable from heartbeats as 'forge unreachable (github_host)' instead of generic stale badges

0 participants